Harden namespace directory ownership and lifecycle cleanup - #36
Open
raphaelfeitoza wants to merge 13 commits into
Open
raphaelfeitoza wants to merge 13 commits into
raphaelfeitoza wants to merge 13 commits into
Conversation
vectorParseSqliteText stores each vector element in a 1025-byte stack buffer whose final byte must remain NUL. The length guard used `>`, so a 1025-character element overwrote that terminator before the guard fired; the following error path then formatted the buffer with `%s`, reading past the end of the stack buffer. Reject the element once it reaches MAX_FLOAT_CHAR_SZ characters so the terminator is preserved, regenerate the bundled amalgamations, and add boundary regression tests for every vector function that parses TEXT.
raphaelfeitoza
added this pull request to stack #37
October 1, 2026 14:49
raphaelfeitoza
marked this pull request as ready for review
October 1, 2026 18:29
Fix out-of-bounds read when formatting overlong vector text elements
libsql-sqlite3/test/rust_suite has no committed Cargo.lock, so CI resolves its transitive dependencies fresh on every run. Several of those now require a newer compiler than the pinned 1.85.0 (icu_* and wasm-encoder/wast declare rust-version 1.88; yoke-derive 0.8.3 declares none but uses str::from_utf8 as an inherent method, stabilized in 1.87). This breaks the Extensions Tests job and the rusttestwasm step of make-sqlite3 on main. Pin current stable (1.98.1) rather than the minimum that compiles today (1.88.0, verified green in CI), so crate MSRV bumps do not break CI again in the near term. Document the constraint next to the unlocked test crate.
CI compiles with RUSTFLAGS="-D warnings", so lints added since 1.85.0 fail the build: - mismatched_lifetime_syntaxes (new in 1.89): eleven signatures elide a lifetime on the input side (&self / &str) but hide it on the output type (Vec<Column>, PageHdrIter, CursorStep<S>, Cow<str>). Spell the output lifetime as '_ as the compiler suggests. No semantic change; this is the lifetime rustc already inferred. - unused_assignments: `frameno` in bottomless-cli's restore loop was only ever copied into BatchReader::new and then incremented, never read. BatchReader tracks its own next_frame_no and the function returns the separate last_received_frame_no, so the local was dead since it was introduced in 4a71b20. Remove it and pass first_frame_no directly. Verified locally on 1.98.1 with the same flags as CI: cargo check --all-targets --all-features, cargo fmt --check, and cargo check -p libsql --no-default-features for core/replication/remote.
Bump pinned Rust toolchain to 1.98.1
raphaelfeitoza
force-pushed
the
fix/namespace-directory-ownership-89265
branch
from
October 1, 2026 19:54
744ca2f to
09e4e6a
Compare
raphaelfeitoza
removed this pull request from stack #37
October 2, 2026 19:35
raphaelfeitoza
added this pull request to stack #40
October 2, 2026 19:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Depends on #31 (
fix/namespace-path-traversal-89265). Review this PR against that branch, notmain; merge #31 first.This extracts directory-ownership and lifecycle hardening from #31; its minimal namespace-string traversal fix remains independently reviewable.
Summary
jobstable.Review and threat model
Independent correctness, security and read-only cross-file reviews found no remaining high/critical issue on final commit
744ca2f75f5cce3141b6400c62ee49fbc4cd1dbbwithin the trusted single-process/process-crash model. A Linux CI replica-reset regression on the preceding commit was fixed by744ca2f75fand the same cluster test passed in the full rerun.This is not a general atomic-filesystem or multi-writer security guarantee. Privileged external filesystem replacement is out of scope. Failed/incomplete resets and quarantined directories may require operator inspection; see
docs/ADMIN_API.md. Sudden power-loss durability on Windows and device-level macOSF_FULLFSYNCare not claimed. Synchronous short filesystem operations under identity coordination avoid cancellation races but can block a Tokio worker on slow filesystems. Logical reopen tests do not substitute for actual SIGKILL/power-loss tests.Validation
cargo fmt --all -- --check,cargo check -p libsql-server --tests --offline,git diff --check: passed.744ca2f75fpassed: both Run Tests jobs, Run Checks, unused-dependency/features check, Windows checks. The first run found a replica-onlyjobstable regression, fixed in this commit; the next run hit aDatabaseBusyfailure in an unchanged concurrent-connection test, passed on retry; a subsequent full rerun passed.libsql_open_v3/sqlite3_*symbols; Linux CI supplies runtime validation.